Audio: Volume: Fix the ramping and ZC mode align#10704
Audio: Volume: Fix the ramping and ZC mode align#10704singalsu wants to merge 2 commits intothesofproject:mainfrom
Conversation
This patch adds to audio_stream.h inline functions: - audio_stream_align_frames_round_down() - audio_stream_align_frames_round_up() They are useful when an algorithm needs to process a number of frames that is smaller then data size than offered by module API. E.g. when volume component ramps the volume, the block with same gain is smaller than a normal processing period. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch fixes an issue where processing smaller parts of the period can break the data align constraints. The frames count value from zc_get() function or from cd->vol_ramp_frames is rounded up to next align compatible frames count. The failure with align constraints appeared as glitches in audio with some 44.1 kHz family sample rates. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
| { | ||
| uint16_t align = stream->runtime_stream_params.align_frame_cnt; | ||
|
|
||
| return (frames / align) * align; |
There was a problem hiding this comment.
can alignment be not a power of 2? We have multiple locations in SOF where we enforce that assumption and then use bit-wise operations apply such power-of-2 alignment. Either way you can use ROUND_DOWN() for the generic case or ALIGN_DOWN() for the power-of-2 case
There was a problem hiding this comment.
The align can be any positive integer number. ROUND_DOWN is some operation with modulo and ALIGN_DOWN is for power of 2. I prefer this simple equation.
There was a problem hiding this comment.
ROUND_DOWN() in Zephyr seems to be same as this but I need to check. See https://docs.zephyrproject.org/latest/doxygen/html/group__sys-util.html#gad8d2389dbe7ea135eccf237dbafb76dd
But in SOF numbers.h it's not done (with modulo) like in Zephyr documentation with basically same equation that I used.
| uint32_t aligned_frames = (frames / align) * align; | ||
|
|
||
| if (aligned_frames < frames) | ||
| aligned_frames += align; |
There was a problem hiding this comment.
I prefer this simple equation to know exactly what it does.
There was a problem hiding this comment.
So, I reject your reject.
There was a problem hiding this comment.
OK, ROUND_UP() looks the the same but it's not in available headers for testbench build (no Zephyr headers), would need to add it.
| frames = cd->zc_get(input_buffers[0].data, cd->vol_ramp_frames, &prev_sum); | ||
| frames = cd->zc_get(source, cd->vol_ramp_frames, &prev_sum); | ||
| /* Align frames count to audio stream constraints */ | ||
| frames = audio_stream_align_frames_round_up(source, frames); |
There was a problem hiding this comment.
is rounding up correct? Wouldn't that grab incomplete frames? Same below. And if you round down then you presumably wouldn't need to add lines 589-590
There was a problem hiding this comment.
A round to nearest (up or down) would be best for ZC but need to check if zero frames need more handling.
2 participants
No description provided.